Repository navigation
Conversation
Resolve rooted absolute exclusions against the cwd drive or UNC share without normalizing glob syntax.
|
This pull request does not currently match the merge queue conditions, so it cannot be queued from here. The box comes back if it matches again. |
kilisamemarisaaa
left a comment
There was a problem hiding this comment.
I independently checked this on Windows / Node 24.12.0 at head 2a2420ed246f0b112b9f2443ff8d16d6d5a6455d, against merge base 2e04f6e55839baa51f4cc4985c58d1a358f7ca80.
Using real temporary files under a directory containing spaces, Chinese characters, literal brackets and braces, I ran 108 public API checks per revision (18 scenarios × relative/absolute output × globSync/glob/globStream):
- Base: 30 pass, 78 fail. All failures are rooted exclusions that leave the unwanted file in the result.
- Head: 108 pass. All 30 passing base controls return exactly the same normalized results on head.
The scenarios cover exact and ignore exclusions, braces/extglobs/character classes, escaped literal filenames, globstar, rooted/drive-qualified positives, relative/URL cwd, hidden files, case-insensitive matching, and nonmatching, relative-negative and drive-qualified controls. Resolving just the root preserves the pattern syntax in these cases; the cwd directory is not prepended to the rest of the rooted pattern.
On the same head, npm run build passed compilation, lint and 292 unit tests; SNAPSHOT_SKIP_PRUNING=1 npm run test:e2e -- --reporter dot passed 1,449 tests, 9 pending. Both source worktrees remained clean. Dependencies were shared between these two isolated checkouts to keep versions identical (TypeScript 6.0.3); no source was patched for the probe. Installation did emit an engine warning for transitive jsdoc-type-pratt-parser@9.4.0 requiring a newer Node patch, though the commands above completed successfully.
These are real local Windows results, not Linux/macOS results or tests against mounted secondary drives/UNC shares. I read the full diff and traced the filter/root-conversion path; this comment establishes the checked behavior, not an exhaustive correctness or cross-platform approval.
AI disclosure: this source review, probe and comment were prepared with AI assistance; the reported commands were executed locally. No independent human review is claimed.
Standalone public-API probe
After compiling both revisions with the same dependencies, save this as probe.mjs and run node probe.mjs <checkout> <report.json> on each checkout. The base exits 1 and head exits 0. Results are normalized relative to the fixture directory; the script removes only its uniquely created temporary directory.
import assert from 'node:assert/strict';
import fs from 'node:fs';
import os from 'node:os';
import path from 'node:path';
import { pathToFileURL } from 'node:url';
const repo = path.resolve(process.argv[2]);
const output = path.resolve(process.argv[3]);
assert.equal(process.platform, 'win32', 'This probe verifies real Windows paths');
const fg = await import(pathToFileURL(path.join(repo, 'out/index.js')).href);
const parent = fs.mkdtempSync(path.join(os.tmpdir(), 'fast-glob-523-'));
const fixture = path.join(parent, 'space [set] {literal} 中文');
fs.mkdirSync(path.join(fixture, 'nested'), { recursive: true });
const files = ['keep.txt', 'drop.txt', '[literal].txt', '.hidden.txt', 'a.js', 'nested/keep.txt', 'nested/drop.txt'];
for (const file of files) fs.writeFileSync(path.join(fixture, file), file);
const absolutePrefix = fg.convertPathToPattern(fixture).replaceAll('\\\\', '/');
const rootedPrefix = '/' + fg.convertPathToPattern(fixture.slice(path.parse(fixture).root.length));
const visibleText = ['[literal].txt', 'drop.txt', 'keep.txt', 'nested/drop.txt', 'nested/keep.txt'].sort();
const allText = [...visibleText, '.hidden.txt'].sort();
const without = (names, excluded) => names.filter(name => !excluded.includes(name));
const cases = [
{ name: 'baseline', expected: visibleText },
{ name: 'rooted-exact', negative: `${rootedPrefix}/drop.txt`, expected: without(visibleText, ['drop.txt']) },
{ name: 'rooted-ignore', ignore: [`${rootedPrefix}/drop.txt`], expected: without(visibleText, ['drop.txt']) },
{ name: 'rooted-braces', negative: `${rootedPrefix}/{drop,missing}.txt`, expected: without(visibleText, ['drop.txt']) },
{ name: 'rooted-extglob', negative: `${rootedPrefix}/@(drop|missing).txt`, expected: without(visibleText, ['drop.txt']) },
{ name: 'rooted-character-class', negative: `${rootedPrefix}/[d]rop.txt`, expected: without(visibleText, ['drop.txt']) },
{ name: 'rooted-escaped-filename', negative: `${rootedPrefix}/\\[literal\\].txt`, expected: without(visibleText, ['[literal].txt']) },
{ name: 'rooted-globstar', negative: `${rootedPrefix}/**/drop.txt`, expected: without(visibleText, ['drop.txt', 'nested/drop.txt']) },
{ name: 'rooted-nonmatch', negative: `${rootedPrefix}/missing.txt`, expected: visibleText },
{ name: 'drive-qualified-negative', negative: `${absolutePrefix}/drop.txt`, expected: without(visibleText, ['drop.txt']) },
{ name: 'relative-negative', negative: 'drop.txt', expected: without(visibleText, ['drop.txt']) },
{ name: 'rooted-positive', positive: `${rootedPrefix}/**/*.txt`, negative: `${rootedPrefix}/drop.txt`, expected: without(visibleText, ['drop.txt']) },
{ name: 'drive-positive', positive: `${absolutePrefix}/**/*.txt`, negative: `${rootedPrefix}/drop.txt`, expected: without(visibleText, ['drop.txt']) },
{ name: 'relative-cwd', cwd: path.relative(process.cwd(), fixture), ignore: [`${rootedPrefix}/drop.txt`], expected: without(visibleText, ['drop.txt']) },
{ name: 'url-cwd', cwd: pathToFileURL(fixture), ignore: [`${rootedPrefix}/drop.txt`], expected: without(visibleText, ['drop.txt']) },
{ name: 'hidden-file', dot: true, negative: `${rootedPrefix}/.hidden.txt`, expected: visibleText },
{ name: 'hidden-control', dot: true, expected: allText },
{ name: 'case-insensitive', caseSensitiveMatch: false, negative: `${rootedPrefix}/DROP.TXT`, expected: without(visibleText, ['drop.txt']) },
];
const results = [];
try {
for (const test of cases) {
for (const absolute of [false, true]) {
const patterns = [test.positive ?? '**/*.txt', ...(test.negative ? ['!' + test.negative] : [])];
const options = {
cwd: test.cwd ?? fixture, absolute, dot: test.dot ?? false,
caseSensitiveMatch: test.caseSensitiveMatch ?? true, ignore: test.ignore ?? [],
};
for (const api of ['globSync', 'glob', 'globStream']) {
let actual;
try {
const returned = api === 'globStream' ? await Array.fromAsync(fg[api](patterns, options)) : await fg[api](patterns, options);
actual = returned.map(value => path.relative(fixture, path.resolve(options.cwd instanceof URL ? fixture : options.cwd, value)).replaceAll('\\', '/')).sort();
assert.deepEqual(actual, test.expected);
results.push({ name: test.name, absolute, api, pass: true, actual });
} catch (error) {
results.push({ name: test.name, absolute, api, pass: false, actual, expected: test.expected, error: error.message });
}
}
}
}
} finally {
// Only remove the unique directory created by this probe.
assert(path.resolve(parent).startsWith(path.resolve(os.tmpdir()) + path.sep));
assert(path.basename(parent).startsWith('fast-glob-523-'));
fs.rmSync(parent, { recursive: true, force: true });
}
const report = { repo, node: process.version, platform: process.platform, total: results.length, passed: results.filter(x => x.pass).length, failed: results.filter(x => !x.pass).length, results };
fs.writeFileSync(output, JSON.stringify(report, null, 2));
console.log(JSON.stringify({ ...report, results: undefined }));
for (const name of new Set(results.filter(x => !x.pass).map(x => x.name))) console.log('FAIL ' + name);
process.exitCode = report.failed ? 1 : 0;
What is the purpose of this pull request?
Fix Windows rooted absolute exclusions so matching positive and negative patterns cancel with or without a drive letter. Fixes #486.
What changes did you make? (Give an overview)
Resolve only the root of single-slash Windows negative patterns against
cwd, preserving glob syntax and explicit drive/UNC roots. Add regression coverage and clarify the Windows pattern documentation.Validation was local to Windows with Node.js 26.3.0. Linux/macOS, other Node.js versions, and physically mounted secondary drives or UNC shares were not exercised.
Implementation and validation
Absolute negative patterns were compiled without a drive while candidate paths were resolved to drive-qualified paths. Prefixing only the escaped filesystem root reconciles these forms without passing glob expressions through
path.resolve.The public
globSyncreproduction returned the fixture before the fix and returns[]after it; the drive-qualified control returns[]in both cases. Before the fix, 7 new unit checks and 54 new sync/async/stream checks failed.npm run build: passes compilation/type checking, lint, and 292 unit tests.SNAPSHOT_SKIP_PRUNING=1 npm run test:e2e -- --reporter dot: 1,449 passing; 9 existing platform-specific pending cases.git diff --check: passes.Coverage includes rooted and drive-qualified positives, negative patterns and
ignore, relative/absolute/URLcwd, relative exclusions, nonmatches, output and case settings, hidden files, trailing directory slashes, braces, extglobs, character classes, escaping, and parent segments. Synthetic entry-filter cases exercise distinct drives, literal UNC share metacharacters, explicit UNC roots, and device-drive roots without accessing other drives or network shares.